Skip to content

feat: auto-derive technique tags from notation in curation - #69

Merged
PhysShell merged 6 commits into
mainfrom
claude/auto-technique-tags
Jun 18, 2026
Merged

feat: auto-derive technique tags from notation in curation#69
PhysShell merged 6 commits into
mainfrom
claude/auto-technique-tags

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Why

Guitar Pro import already records techniques explicitly (ADR-0018), yet curation
left ChunkMeta.techniques empty and asked a human to re-tag by hand what the
tab already states. This is the first step of corpus-curation automation: read
the techniques for free.

What

New griff_core::technique:

  • derive_techniques(score, track) — scans the track's primary voice
    (voice 0)
    — the same convention structure/gesture/boundaries follow —
    and returns the techniques the notation states are present:
    • tags: the SwancoreTags with a dedicated per-occurrence variant —
      hammer-on, pull-off, slide, bend, vibrato, palm-mute, natural/artificial
      harmonic;
    • names: the superset of lower_snake_case technique names, also
      recording legato, accent, ghost, staccato, dead-note and tap (which have no
      dedicated tag).
  • merge_tags(chosen, derived) — keeps the curator's order, then appends
    any derived tag not already present. Stable and idempotent, so auto-derivation
    adds technique tags without overriding the curator's choices.

Wired into both chunk-assembly seams — CLI griff curate
(build_chunk_meta) and the web capture tool (build_chunk_meta_record):
techniques is now populated and the tags are the union of the curator's picks
and the derived set. The emitted chunk.json stays byte-compatible with what
griff manifest reads.

Deliberately out of scope

Presence-only — no thresholds or heuristics — so it stays a pure function of the
score (SPEC §6). The passage-level tags TappingPassage / LegatoPassage are
about dominance, not presence, so they are intentionally not derived here
(a single legato slur is not a legato passage). Candidate for a follow-up.

Tests (strict TDD)

  • 298aea9 red → 719557e green for derive_techniques; the green step also
    pins the voice-0 convention (a technique in a secondary voice is not derived),
    caught from the CLI's primary_voice_note_count tests.
  • core/tests/technique_tags.rs: mapping, dedupe/determinism, primary-voice,
    merge_tags order + idempotency.
  • CLI + web wiring each get a test that a techniques-bearing track auto-fills
    techniques and merges tags while keeping the curator's hand-picked tag.

Validation

  • core 90 unit + 5 technique_tags; cli unit (incl. new wiring test) +
    curate_cmd 10 unchanged; web 17/17 host tests.
  • cargo clippy --workspace --all-targets -- -D warnings (incl. nursery)
    clean; web crate clippy clean; wasm32-unknown-unknown release build green.

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Auto-detect techniques from Guitar Pro notation during chunk curation
    • Automatically populate chunk techniques based on notation spans and note marks
    • Preserve curator-selected tags while merging with auto-detected technique tags
  • Tests
    • Added unit tests covering technique derivation, deterministic ordering, deduplication, out-of-range behavior, and tag merge idempotency

claude added 5 commits June 18, 2026 10:52
Guitar Pro import already records techniques explicitly (ADR-0018), so
curation should read them instead of re-asking a human. Pins
derive_techniques(score, track): spans/marks with a dedicated SwancoreTag
become tags (hammer-on, palm-mute, pinch→artificial harmonic, …); the
free-form `techniques` list is the superset (legato, accent, … too).

References griff_core::technique, which does not exist yet — the suite fails
to compile until the green step.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
griff_core::technique::derive_techniques(score, track) reads the track's
primary voice (voice 0 — the convention structure/gesture/boundaries follow)
and returns the techniques its notation states are present:
- tags: SwancoreTags with a dedicated per-occurrence variant (hammer-on,
  pull-off, slide, bend, vibrato, palm-mute, natural/artificial harmonic);
- names: the superset of lower_snake_case names, also recording legato,
  accent, ghost, staccato, dead-note and tap, which have no dedicated tag.

Presence-only (no thresholds), so it stays a pure function of the score
(SPEC §6). The passage-level TappingPassage/LegatoPassage tags are about
dominance, not presence, and are intentionally left out. 4/4 technique_tags
tests pass; clippy -D warnings (incl. nursery) clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Keeps the curator's order, then appends any derived tag not already present.
Stable and idempotent, so auto-derivation adds technique tags without
overriding choices. The shared seam both `griff curate` and the web capture
tool use to fold derive_techniques output into ChunkMeta.tags.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
build_chunk_meta now derives techniques from the selected track's notation
(technique::derive_techniques) and folds them in: ChunkMeta.techniques is the
free-form name list (was always empty), and tags merge the curator's choices
with the derived technique tags (technique::merge_tags). The curator no longer
re-tags by hand what the Guitar Pro file already states.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
build_chunk_meta_record now folds technique::derive_techniques into the chunk:
`techniques` carries the free-form name list (was always empty) and the tags
merge the curator's checkbox picks with the derived technique tags. The emitted
chunk.json stays byte-compatible with what `griff manifest` reads. 17/17 web
tests pass; wasm32 release build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 14311d3f-dee4-4daa-ab00-21069ec35bb6

📥 Commits

Reviewing files that changed from the base of the PR and between eec7e70 and d52a7cb.

📒 Files selected for processing (2)
  • core/src/technique.rs
  • core/tests/technique_tags.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • core/src/technique.rs

📝 Walkthrough

Walkthrough

Adds a new core/src/technique.rs module exposing DerivedTechniques, derive_techniques, and merge_tags. Both the CLI (build_chunk_meta) and web (build_chunk_meta_record) chunk curation paths now call this module to auto-populate ChunkMeta.techniques and merge derived technique tags with curator-chosen tags.

Changes

Technique auto-derivation into ChunkMeta

Layer / File(s) Summary
Core technique module: struct, mappings, and functions
core/src/lib.rs, core/src/technique.rs
Registers pub mod technique, defines DerivedTechniques { tags, names }, adds canonical SpanTechnique/NoteMark-to-SwancoreTag mapping helpers, implements derive_techniques (multi-voice scan with ordered deduplication), and implements merge_tags (stable union appending only missing derived tags).
Core technique tests
core/tests/technique_tags.rs
Adds score_with minimal score builder and five test cases: span-to-tag mapping with names superset, determinism/deduplication, empty/out-of-range track default, merge_tags order preservation and idempotence, secondary-voice-only technique detection.
CLI and web integration
cli/src/main.rs, web/src/lib.rs
Both entry points import technique, call derive_techniques for the selected track, merge derived tags with curator-chosen tags via merge_tags, and set ChunkMeta.techniques from derived names instead of an empty list; integration tests are added for both.

Sequence Diagram(s)

sequenceDiagram
  participant Curator
  participant build_chunk_meta
  participant derive_techniques
  participant merge_tags
  participant ChunkMeta

  Curator->>build_chunk_meta: inputs (track_index, chosen_tag_indices, score)
  build_chunk_meta->>derive_techniques: (score, track_index)
  derive_techniques-->>build_chunk_meta: DerivedTechniques {tags, names}
  build_chunk_meta->>merge_tags: (chosen_tags, derived.tags)
  merge_tags-->>build_chunk_meta: merged Vec<SwancoreTag>
  build_chunk_meta->>ChunkMeta: techniques = derived.names, tags = merged
  ChunkMeta-->>Curator: populated ChunkMeta record
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PhysShell/griff#51: Updates the GP importer to emit additional TechniqueSpans (e.g., Bend) and NoteMark variants (e.g., DeadNote), which directly extends the set of techniques that derive_techniques can detect and auto-populate into ChunkMeta.techniques.
  • PhysShell/griff#67: The retrieved PR introduces build_chunk_json for the chunk.json flow in web/src/lib.rs, while this PR updates the metadata-building checkpoint (build_chunk_meta_record) that both paths share to auto-derive techniques and merge technique tags.

Poem

🐰 Hop, hop, through the staff I go,
Hammer-ons and harmonics in a row,
Each span I sniff, each mark I find,
No technique left for me behind!
The ChunkMeta blooms with names galore—
One happy rabbit, tagging more! 🎸

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: auto-derive technique tags from notation in curation' directly and clearly summarizes the main change: automatic derivation of technique tags from Guitar Pro notation during curation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/auto-technique-tags

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Owner Author

@codex review


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eec7e70109

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread core/src/technique.rs Outdated
Codex P2 (#69): derive_techniques scanned track.voices.first(), but
track_notes/technique_share (structure + complexity.technical) measure a track
as a whole across every voice — so a chunk could count a secondary-voice
technique in complexity.technical while its `techniques`/tags metadata omitted
it. Scan every voice to match, the same voice-handling decision reached in #38.

Flips the voice test to assert a secondary-voice technique IS derived.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95

Copy link
Copy Markdown
Owner Author

@codex review

Addressed the P2 — derive_techniques now scans all voices (d52a7cb), matching track_notes/technique_share (structure + complexity.technical), so the techniques/tags metadata can no longer disagree with the technical metric. Same voice-handling call reached in #38. Good catch. 🙏


Generated by Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: d52a7cb2cc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants